Surface MCP OAuth reauthorization during catalog discovery - #1818
Merged
RhysSullivan merged 8 commits intoAug 29, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the MCP plugin + core tool-sync path so that when an OAuth-backed MCP server rejects a previously stored bearer with 401, catalog discovery stops at the authenticated HTTP boundary and surfaces a structured “reauthorization required” outcome. Core can now preserve the last authoritative tool catalog while persisting an actionable expired health verdict to drive reconnect UX, and it avoids triggering the MCP SDK’s OAuth metadata discovery / Dynamic Client Registration during background refresh.
Changes:
- Add a
staticOAuthBearertransport flag to treat401asMcpOAuthReauthorizationRequired(preventing SDK OAuth fallback + DCR during discovery). - Thread a structured
reauthorizationRequiredsignal through discovery errors and map it tohealth: { status: "expired" }inresolveTools. - Extend core’s
ResolveToolsResultto optionally carryhealth, and persist that health during tool sync; add regression tests and a changeset.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugins/mcp/src/sdk/plugin.ts | Marks OAuth-backed remote transports as staticOAuthBearer and surfaces reauth-required discovery as expired health in resolveTools. |
| packages/plugins/mcp/src/sdk/plugin.test.ts | Adds regression coverage ensuring rejected OAuth bearer surfaces expired health and avoids DCR. |
| packages/plugins/mcp/src/sdk/errors.ts | Extends McpToolDiscoveryError with an optional reauthorizationRequired structural signal. |
| packages/plugins/mcp/src/sdk/discover.ts | Maps McpOAuthReauthorizationRequired into McpToolDiscoveryError.reauthorizationRequired. |
| packages/plugins/mcp/src/sdk/connection.ts | Implements staticOAuthBearer handling: throw typed reauth error on 401, and improve nested-cause detection. |
| packages/core/sdk/src/plugin.ts | Adds optional health?: HealthCheckResult to ResolveToolsResult so plugins can surface actionable outcomes during discovery. |
| packages/core/sdk/src/executor.ts | Persists result.health (when provided) while preserving prior catalogs on incomplete discovery. |
| packages/core/sdk/src/executor.test.ts | Adds a regression test verifying actionable health survives connections.refresh. |
| .changeset/mcp-oauth-refresh-reauthorization.md | Documents the patch-level behavior change for SDK + MCP plugin packages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| detail: expect.stringContaining("reauthorization"), | ||
| }, | ||
| }); | ||
| expect(ledger.requests.filter((url) => new URL(url).pathname === "/register")).toEqual([]); |
Comment on lines
+45
to
+48
| /** The MCP OAuth provider reached the interactive authorization boundary. | ||
| * Catalog callers use this structural signal to request reconnect without | ||
| * parsing or exposing an upstream error message. */ | ||
| reauthorizationRequired: Schema.optional(Schema.Boolean), |
…ing catalog discovery
…ol-refresh-reauth # Conflicts: # packages/plugins/mcp/src/sdk/discover.ts # packages/plugins/mcp/src/sdk/errors.ts
…ol-refresh-reauth
This was referenced Aug 29, 2026
Merged
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported issue
An OAuth-backed remote MCP can reject Executor's stored bearer before the locally stored expiry. During catalog discovery, Executor handed that
401to the MCP SDK's interactive OAuth fallback. A background refresh couldn't finish authorization, but the SDK first performed OAuth metadata discovery and Dynamic Client Registration. Executor then preserved the previous catalog under a generic degraded health result, leaving clients with zero or stale tools and no reliable reconnect signal.Fix
OAuth-backed MCP transports now stop at the authenticated HTTP boundary when the resolved bearer gets a
401. The connector carries a structured reauthorization error through discovery instead of allowing the SDK to start an interactive fallback or register a disposable client.Incomplete catalog results can now carry an actionable health result. Core preserves the last authoritative catalog while persisting the MCP connection as expired with a reconnect-required detail. Generic incomplete discovery remains degraded, and API-key or unauthenticated
401behavior is unchanged.The regression coverage exercises the real rejected-bearer path, asserts that no registration request occurs, and proves the expired verdict survives
connections.refresh. The MCP plugin suite, core SDK suite, lint, formatting, and affected-package type checks pass locally.Fixes #1816
🤖 This content was generated with AI assistance using GPT-5.6 Sol via Hermes Agent.